Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Jul 9, 2025

Fixes #5448

This PR fixes a critical security issue where mode restrictions were being bypassed when auto-approval was enabled.

The previous fix for auto-approval introduced a regression where FileRestrictionError exceptions were ignored when auto-approval was enabled. This allowed modes with file restrictions (like Architect mode) to write to any file type.

This fix ensures that all validation errors, including FileRestrictionError, are treated consistently and always block tool execution. Auto-approval should only apply to tools that pass validation, not bypass validation entirely.

Changes:

  • Removed special handling for FileRestrictionError in presentAssistantMessage.ts
  • All validation errors now consistently block tool execution
  • Added comprehensive tests to verify mode restrictions are enforced with auto-approval

Tests added:

  • Verify file restriction errors block execution even with auto-approval enabled
  • Verify file restriction errors block execution when auto-approval is disabled
  • Verify non-FileRestrictionError errors are always blocked
  • Verify auto-approved tools work correctly when validation passes

Important

Enforces mode restrictions in presentAssistantMessage.ts by blocking all validation errors, including FileRestrictionError, even with auto-approval enabled, and adds tests to verify this behavior.

  • Behavior:
    • Enforces mode restrictions in presentAssistantMessage.ts even with auto-approval enabled by removing special handling for FileRestrictionError.
    • All validation errors now block tool execution, ensuring auto-approval only applies to tools that pass validation.
  • Tests:
    • Added presentAssistantMessage.auto-approve.spec.ts to verify file restriction errors block execution with and without auto-approval.
    • Tests ensure non-FileRestrictionError errors are always blocked and auto-approved tools work correctly when validation passes.

This description was created by Ellipsis for adc4d0b. You can customize this summary. It will automatically update as commits are pushed.

- Remove special handling for FileRestrictionError when auto-approval is enabled
- Ensure all validation errors block tool execution consistently
- Add tests to verify mode restrictions are enforced with auto-approval
- Fixes issue where Architect mode could write any file type when auto-approval was on
@daniel-lxs daniel-lxs requested review from cte, jr and mrubens as code owners July 9, 2025 13:31
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jul 9, 2025
@delve-auditor
Copy link

delve-auditor bot commented Jul 9, 2025

No security or compliance issues detected. Reviewed everything up to adc4d0b.

Security Overview
  • 🔎 Scanned files: 2 changed file(s)
Detected Code Changes
Change Type Relevant files
Enhancement ► presentAssistantMessage.ts
    Update tool validation handling with auto-approval
► presentAssistantMessage.auto-approve.spec.ts
    Add tests for mode restrictions with auto-approval
Bug Fix ► presentAssistantMessage.ts
    Enforce mode restrictions with auto-approval enabled

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 9, 2025
@daniel-lxs daniel-lxs closed this Jul 9, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 9, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Roo Code secrets should respect VSCode Profiles

3 participants